chore: init plan validation schema#1092
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds database schema and migrations for deployment plan validation results and OPA policy rules. It introduces two new tables ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 54 minutes and 28 seconds.Comment |
There was a problem hiding this comment.
Pull request overview
Introduces the initial database schema needed to persist “plan validation” rules (OPA/Rego) and per-target validation results, as part of the DB layer for issue #1088.
Changes:
- Adds
policy_rule_plan_validation_opaanddeployment_plan_target_result_validationtables to the Drizzle schema (plus relations fromdeploymentPlanTargetResult). - Adds a new Drizzle migration (
0193_adorable_paibok.sql) and updates Drizzle metadata (journal + snapshot).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/db/src/schema/deployment-plan.ts | Adds new plan-validation rule/result tables and wires relations from deployment plan target results. |
| packages/db/drizzle/0193_adorable_paibok.sql | Creates the new tables, indexes, and foreign keys for the plan-validation schema. |
| packages/db/drizzle/meta/_journal.json | Registers migration 0193_adorable_paibok in the Drizzle journal. |
| packages/db/drizzle/meta/0193_snapshot.json | Captures the updated schema snapshot after adding the new tables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: text("name").notNull(), | ||
| description: text("description"), | ||
| rego: text("rego").notNull(), | ||
| severity: text("severity").notNull(), | ||
| createdAt: timestamp("created_at", { withTimezone: true }) | ||
| .notNull() | ||
| .defaultNow(), |
|
|
||
| export const deploymentPlanTargetResultValidation = pgTable( | ||
| "deployment_plan_target_result_validation", | ||
| { | ||
| id: uuid("id").primaryKey().defaultRandom(), | ||
| resultId: uuid("result_id") | ||
| .notNull() | ||
| .references(() => deploymentPlanTargetResult.id, { onDelete: "cascade" }), | ||
| ruleId: uuid("rule_id").notNull(), | ||
| passed: boolean("passed").notNull(), | ||
| violations: jsonb("violations").$type<Violation[]>().notNull().default([]), | ||
| evaluatedAt: timestamp("evaluated_at", { withTimezone: true }) | ||
| .notNull() |
| "created_at" timestamp with time zone DEFAULT now() NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| ALTER TABLE "deployment_plan_target_result_validation" ADD CONSTRAINT "deployment_plan_target_result_validation_result_id_deployment_plan_target_result_id_fk" FOREIGN KEY ("result_id") REFERENCES "public"."deployment_plan_target_result"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint |
| .notNull() | ||
| .references(() => deploymentPlanTargetResult.id, { onDelete: "cascade" }), | ||
| ruleId: uuid("rule_id").notNull(), | ||
| passed: boolean("passed").notNull(), |
| export const policyRulePlanValidationOpa = pgTable( | ||
| "policy_rule_plan_validation_opa", | ||
| { | ||
| id: uuid("id").primaryKey().defaultRandom(), | ||
| policyId: uuid("policy_id") | ||
| .notNull() | ||
| .references(() => policy.id, { onDelete: "cascade" }), | ||
| name: text("name").notNull(), | ||
| description: text("description"), |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/db/src/schema/deployment-plan.ts`:
- Line 229: Change the bare UUID column declaration for ruleId into a proper FK
by making ruleId reference the rules table primary key (e.g., replace
uuid("rule_id").notNull() with a declaration that includes .references(...) /
foreign key to rules.id) so the DB enforces referential integrity; also ensure
the ORM relation defined around the existing relation block (the relation on
lines ~246-248) matches the FK, then regenerate the migration so the SQL
includes the rule_id foreign key constraint.
- Around line 191-207: The new table policyRulePlanValidationOpa is missing the
tenant-scoping workspaceId column; add a workspaceId field (e.g.,
uuid("workspace_id").notNull().references(() => workspace.id, { onDelete:
"cascade" })) to the pgTable definition, include it in the index tuple (e.g.,
(t) => [index().on(t.workspaceId), index().on(t.policyId)]) or a composite index
if desired, and apply the same change to the other new table(s) in this file
(the second pgTable block mentioned) so all tables include workspaceId for
multi-tenant isolation.
- Line 201: The column definition severity: text("severity").notNull() currently
allows any string; change it to a strict enum type and update imports so only
"error" or "warning" are permitted at the schema level (e.g., replace the text
column with a Postgres enum creation via pgEnum/enum helper or a DB-level check
constraint and import the enum helper). Update the symbol severity in
deployment-plan.ts to use that enum (and ensure the enum values are
["error","warning"]) so migrations/schemas enforce the allowed values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3824504e-081a-4302-adb7-47cb064b7f74
📒 Files selected for processing (4)
packages/db/drizzle/0193_adorable_paibok.sqlpackages/db/drizzle/meta/0193_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/deployment-plan.ts
| export const policyRulePlanValidationOpa = pgTable( | ||
| "policy_rule_plan_validation_opa", | ||
| { | ||
| id: uuid("id").primaryKey().defaultRandom(), | ||
| policyId: uuid("policy_id") | ||
| .notNull() | ||
| .references(() => policy.id, { onDelete: "cascade" }), | ||
| name: text("name").notNull(), | ||
| description: text("description"), | ||
| rego: text("rego").notNull(), | ||
| severity: text("severity").notNull(), | ||
| createdAt: timestamp("created_at", { withTimezone: true }) | ||
| .notNull() | ||
| .defaultNow(), | ||
| }, | ||
| (t) => [index().on(t.policyId)], | ||
| ); |
There was a problem hiding this comment.
New tables are missing workspaceId for tenant isolation.
Both newly added tables omit a workspaceId column, which weakens direct tenant scoping and violates the repository’s multi-tenant schema rule.
As per coding guidelines **/packages/db/**/*.ts: All database tables include workspaceId for multi-tenant isolation.
Also applies to: 222-237
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/src/schema/deployment-plan.ts` around lines 191 - 207, The new
table policyRulePlanValidationOpa is missing the tenant-scoping workspaceId
column; add a workspaceId field (e.g.,
uuid("workspace_id").notNull().references(() => workspace.id, { onDelete:
"cascade" })) to the pgTable definition, include it in the index tuple (e.g.,
(t) => [index().on(t.workspaceId), index().on(t.policyId)]) or a composite index
if desired, and apply the same change to the other new table(s) in this file
(the second pgTable block mentioned) so all tables include workspaceId for
multi-tenant isolation.
| name: text("name").notNull(), | ||
| description: text("description"), | ||
| rego: text("rego").notNull(), | ||
| severity: text("severity").notNull(), |
There was a problem hiding this comment.
Constrain severity to allowed values at schema level.
Line 201 currently accepts any text; this should be restricted to "error" or "warning" to prevent invalid rule configurations entering the DB.
Suggested fix
+export const policyRulePlanValidationSeverity = pgEnum(
+ "policy_rule_plan_validation_severity",
+ ["error", "warning"],
+);
export const policyRulePlanValidationOpa = pgTable(
"policy_rule_plan_validation_opa",
{
@@
- severity: text("severity").notNull(),
+ severity: policyRulePlanValidationSeverity("severity").notNull(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/src/schema/deployment-plan.ts` at line 201, The column definition
severity: text("severity").notNull() currently allows any string; change it to a
strict enum type and update imports so only "error" or "warning" are permitted
at the schema level (e.g., replace the text column with a Postgres enum creation
via pgEnum/enum helper or a DB-level check constraint and import the enum
helper). Update the symbol severity in deployment-plan.ts to use that enum (and
ensure the enum values are ["error","warning"]) so migrations/schemas enforce
the allowed values.
| resultId: uuid("result_id") | ||
| .notNull() | ||
| .references(() => deploymentPlanTargetResult.id, { onDelete: "cascade" }), | ||
| ruleId: uuid("rule_id").notNull(), |
There was a problem hiding this comment.
Add the missing foreign key on ruleId.
Line 229 defines ruleId as a bare UUID, so the DB can store validation rows pointing to non-existent rules. The relation on Lines 246-248 is ORM-level only and does not enforce referential integrity.
Suggested fix
export const deploymentPlanTargetResultValidation = pgTable(
"deployment_plan_target_result_validation",
{
@@
- ruleId: uuid("rule_id").notNull(),
+ ruleId: uuid("rule_id")
+ .notNull()
+ .references(() => policyRulePlanValidationOpa.id, { onDelete: "cascade" }),After this, regenerate the migration so SQL includes the rule_id FK.
Also applies to: 246-248
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/src/schema/deployment-plan.ts` at line 229, Change the bare UUID
column declaration for ruleId into a proper FK by making ruleId reference the
rules table primary key (e.g., replace uuid("rule_id").notNull() with a
declaration that includes .references(...) / foreign key to rules.id) so the DB
enforces referential integrity; also ensure the ORM relation defined around the
existing relation block (the relation on lines ~246-248) matches the FK, then
regenerate the migration so the SQL includes the rule_id foreign key constraint.
fixes #1088
Summary by CodeRabbit
Release Notes